I'm not very familiar with bower, but we would need to get https://unpkg.com/xterm@~3.0.1/dist/xterm.css as well.

You may be able to get away with inventing a fake "xtermjs-css" package and using that.

This suffers from the same problem that is holding up JupyterLab: we can't specify the padding yet. You could work around it here by adding an extra wrapper div so that the terminal host div is already inset from the wrapper.

cf jupyterlab/jupyterlab#3553, xtermjs/xterm.js#946

For reference, on master:

@blink1073 couldn't you just wrap the element you open xterm in inside a container with some padding? Does this not work? <div style="padding: 10px"><div id="term"></div></div>

Yep, that's what I recommended above 😉. We didn't do that in JupyterLab because we were about to have a major release and there were a few other things that were not quite working yet in the changeover.

@blink1073 oh ok, I thought that was a blocker for you

Ok, so what to do about this PR?

@cancan101, I'd say wrap this element as @Tyriar recommended above:

https://github.com/cancan101/notebook/blob/131502de3be2e6d4b12da2d93303e2cfb25409ea/notebook/templates/terminal.html#L26

I'm not sure that fix works. Maybe we have to wait for xtermjs/xterm.js#1123?

@cancan101, why wouldn't wrapping the div work? It might also take updating some CSS/LESS.

Ah, I see what you mean. It is more invasive than I'd hoped. Yeah, I think we should wait for the upstream fix.

xtermjs/xterm.js#1208 has been merged, so I guess we're just waiting for xtermjs 3.1.0 to be released.

I bumped the version pin. Now I just need to set the padding.

Just pulling in 3.1.0 but without making any CSS changes, I get:

The left / top look okay, but something is overflowing on the right.

EDIT
The height of the container also appears to be to large. It looks like the calculated number of rows does not match what actually fits.

Is this and this really the best way to calculate the number of columns / rows: (CC @Tyriar )?

e.g. this is the hack fix I am using:

diff --git a/notebook/static/terminal/js/main.js b/notebook/static/terminal/js/main.js
index 92b160753..4dbc3f55f 100644
--- a/notebook/static/terminal/js/main.js
+++ b/notebook/static/terminal/js/main.js
@@ -50,8 +50,8 @@ requirejs([
     function calculate_size() {
         var height = $(window).height() - header.offsetHeight;
         var width = $('#terminado-container').width();
-        var rows = Math.min(1000, Math.max(20, Math.floor(height/termRowHeight())-1));
-        var cols = Math.min(1000, Math.max(40, Math.floor(width/termColWidth())-1));
+        var rows = Math.min(1000, Math.max(20, Math.floor(height/termRowHeight())-1))-7;
+        var cols = Math.min(1000, Math.max(40, Math.floor(width/termColWidth())-1))-7;
         console.log("resize to :", rows , 'rows by ', cols, 'columns');
         return {rows: rows, cols: cols};
     }

EDIT2
Okay, I think the above approach to calculate the number of rows / columns will no longer work due to the "New Canvas-based Rendering Engine" (ie no longer using dom elements).

EDIT3
This looks quite relevant: https://xtermjs.org/docs/api/addons/fit/ ;-)

Yes resizing is a little more involved now, please report if the fit addon doesn't meet your needs.

Ok, I think it's working now:

@takluyver ok reverted those and added some comments.

@Tyriar ran into this issue: xtermjs/xterm.js#1283 . worked around it for the time being.

Looks like tests failed since "PhantomJS has crashed".

Thanks! The tests passed after I restarted the relevant job, and I've tried it manually.